Fix test-webgpu-native: export the missing rope fixture, raise the job timeout - #21690
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/21690
Note: Links to docs will display an error until the docs builds have been completed. ⏳ No Failures, 82 PendingAs of commit e41f218 with merge base 9cd0c12 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This PR needs a
|
How this was tested
The job is still red, for an unrelated reasonOnce the C++ tests pass, the job continues into op test generation and stops there: This pull request does not cause that and does not claim to fix it. The In other words this is the first of two fixes. It stands on its own and is worth landing on its own, and the batch of 2 conv2d case needs a separate follow up from someone who owns the Vulkan partitioner. |
3f99b6c to
2df108d
Compare
2df108d to
9a0df85
Compare
9a0df85 to
e41f218
Compare
…#21691) ## What is broken Every so often a commit on main shows a cancelled ARM or CoreML job on HUD even though nobody cancelled anything and the commit is fine. It looks like a real failure, so the oncall investigates a phantom. ## Why it is broken GitHub Actions cancels an older run when a newer run lands in the same concurrency group. These workflows use a group key built like this: ``` group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref_name }}-${{ github.ref_type == 'branch' && github.sha }}-${{ github.event_name == 'workflow_dispatch' }} cancel-in-progress: true ``` The key already tells apart pull requests, branches, commits and manual runs. It does not tell apart a scheduled run. These same workflows also have a nightly `schedule` trigger. When the nightly cron fires, `github.ref_name` is `main` and `github.sha` is the current head of main, which are exactly the same values the push run for that commit used. Same key, `cancel-in-progress: true`, so the cron run kills the push run that was still going. The result is a commit whose CI reads as cancelled with no explanation. ## The fix Add `-${{ github.event_name == 'schedule' }}` to the concurrency group, so a scheduled run and a push run on the same commit sit in different groups and no longer cancel each other. This is the same pattern already used by other workflows in this repo. It is applied here to every workflow that has both a `schedule` and a `push` trigger: - `test-backend-arm.yml` - `test-backend-coreml.yml` - `test-backend-openvino.yml` - `test-backend-qnn.yml` - `test-backend-vulkan.yml` - `test-backend-webgpu.yml` - `test-backend-xnnpack.yml` - `test-webgpu-native.yml` - `build-cmsis-pack.yml` - `docker-builds.yml` `doc-build.yml` also has both triggers but is deliberately left alone: it sets `cancel-in-progress: ${{ github.event_name == 'pull_request' }}`, so outside a pull request it never cancels anything and the collision cannot happen there. This only separates schedule from push. It does not change how two scheduled runs of the same workflow behave: `github.sha` is still in the key, so two scheduled runs share a group only while `main` has not moved between them, and that is unchanged by this PR. ## How this was verified Every workflow under `.github/workflows` with a `schedule:` trigger was listed together with its concurrency group, to make sure the change is both correct and complete: - `apple.yml`, `periodic.yml` and `riscv64.yml` already carry exactly the `-${{ github.event_name == 'schedule' }}` suffix this PR adds, so this is not a new idea, it is existing practice in this repo. `periodic.yml` goes one step further and also appends `${{ github.event.schedule }}` to separate its individual crons. - The 10 workflows changed here are precisely the ones still on the older key that also have a `push` trigger, so none were missed. - `build-cadence-runner.yml` and `test-pico2-build.yml` already put `${{ github.event_name }}` in the key, which covers the same collision. - `nightly.yml` has both triggers but only pushes on `ciflow/nightly/*` tags, so its `github.ref_name` is a tag and can never match the scheduled run on `main`. - `doc-build.yml` is the case described above. - The other scheduled workflows have no `concurrency` block at all. Note that the fix only takes effect once it is on `main`, because a scheduled run always uses the workflow file from the default branch. CI on this pull request: 213 checks reported, 195 success, 15 skipped, 2 cancelled, 0 failures. Both cancellations were manual, and in one of them every step had already finished successfully before the cancel landed. ## Overlap with other pull requests Two of the files here are also touched by other open pull requests, in both cases far away from the `concurrency:` block this PR edits: - `.github/workflows/test-webgpu-native.yml` also gets a `timeout:` change in #21690, in the job block further down. - `.github/workflows/test-backend-coreml.yml` also gets a matrix `exclude:` entry in #21695, likewise further down. The hunks do not touch and the changes merge cleanly in any order.
What is broken
test-webgpu-nativeis red on main. The failing test isWebGPUNative.RopeHfDynamicSequenceReusedGraphand it fails withError::AccessFailed, which is what ExecuTorch returns when it cannot open a.ptefile.A second, quieter problem in the same job: its 120 minute limit does not cover the slow end of its own setup. That is not what is red today, but it is close enough to the edge to be worth fixing in the same change. Details below.
Why the test is broken
The C++ test loads a model file named
rope_hf_dynamic_sequence.pteout of the directory given by theWEBGPU_TEST_ROPE_HF_DIRenvironment variable.The CI script that prepares the test fixtures,
backends/webgpu/scripts/test_webgpu_native_ci.sh, only calledexport_rope_hf_dynamic(...). It never calledexport_rope_hf_dynamic_sequence(...), sorope_hf_dynamic_sequence.ptewas never written. The test then tried to open a file that did not exist.The Python helper
export_rope_hf_dynamic_sequencealready exists inbackends/webgpu/test/ops/test_rope_hf.py. It was just never called from CI.The contract test that is supposed to catch exactly this kind of gap only checked for the other fixture, so nothing failed at lint time either.
The fix
export_rope_hf_dynamic_sequence('${ROPE_HF_DIR}')in the CI script, right next to the existingexport_rope_hf_dynamiccall.require_filecheck for each of the two.ptefiles. If a fixture ever goes missing again, the script now stops early with a readable message instead of failing deep inside a C++ test with a numeric error code.test_native_ci_contract.pyso it asserts that both exports and bothrequire_filechecks are present.timeoutfrom 120 to 150 minutes.No test logic and no backend code changed. Items 1 to 3 only produce a file the test always expected to find.
Why the timeout has to go up
The relevant numbers, all from
test-webgpu-nativeruns on the samelinux.4xlarge.memoryrunner label:The test script is the stable part. The setup in front of it is not: the Dawn plus SwiftShader source build alone has been measured at 9.7, 44.0 and 55.9 minutes. The dependency versions are pinned and the build is the same one every time, so this is runner variance, not anything the repo controls.
The two slow runs are short on the test side only because they were before #21646, when the op-test stage was still wrapped in a warning and skipped. With today's script the test side is the 57.4 minute figure. Put the worst setup we have observed in front of it and the job needs 121 minutes, which is already over the current 120 limit.
150 leaves real headroom without letting a genuinely hung job sit for hours.
How this was verified
test-webgpu-nativewas dispatched on this branch. In that runWebGPUNative.RopeHfDynamicSequenceReusedGraphpasses and the script prints=== WebGPU native tests on Dawn: all run targets passed ===, so the fixture isnow produced and the test that is red on main is green.
On its own this branch does not make the whole job green: the script then reaches
a separate op-test stage that fails on an unrelated problem in the op-test
generator, which #21697 fixes. The two were therefore also tested together, on a
branch holding both changes, and that run passes end to end:
So this PR plus #21697 turn
test-webgpu-nativegreen. Either one alone is notenough, and they are separate problems, so they are separate changes.
Overlap with other pull requests
.github/workflows/test-webgpu-native.ymlis also edited by #21691, which changesthe
concurrency:block near the top of the file. This PR changestimeout:inthe job block further down. The hunks do not touch, and the two merge cleanly in
either order.